Fix fread clipboard handling on Windows (fixes #1292)#7640
Fix fread clipboard handling on Windows (fixes #1292)#7640AmanKashyap0807 wants to merge 8 commits intoRdatatable:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7640 +/- ##
==========================================
- Coverage 99.02% 98.85% -0.18%
==========================================
Files 87 87
Lines 16897 16928 +31
==========================================
+ Hits 16733 16734 +1
- Misses 164 194 +30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @MichaelChirico, I’ve synced the branch with master. Let me know if there’s anything else I should do on my end to help resolve these! |
R/fread.R
Outdated
| } else if (grepl("^clipboard(-[0-9]+)?$", tolower(input))) { | ||
| is_windows = identical(.Platform$OS.type, "windows") | ||
| if (is_windows) { | ||
| clip = tryCatch(utils::readClipboard(), error = identity) |
There was a problem hiding this comment.
we don't normally use namespacing for functions from utils, see e.g. head/tail
There was a problem hiding this comment.
we don't have blanket utils import so it'll need to be included:
Line 148 in 1bd88cb
There was a problem hiding this comment.
readClipboard() is documented as Windows‑only in the utils::clipboard help:
https://search.r-project.org/R/refmans/utils/help/clipboard.html
There are also reports on non‑Windows where readClipboard() simply isn’t available even with utils loaded,
(https://stackoverflow.com/questions/24365249/readclipboard-removed-from-utils-package)
so importing it unconditionally in NAMESPACE risks errors when the package is loaded on those platforms. That’s why I kept utils::readClipboard() inside the Windows guard.
Regarding head/tail, data.table::last() has a small fallback that explicitly calls utils::head and utils::tail instead of using them unqualified everywhere, and that pattern was in my mind when I treated utils::readClipboard() as a similar Windows‑specific special case. I’m happy to change this to plain readClipboard() with a NAMESPACE import if you prefer and are confident it behaves well on non‑Windows platforms.
|
I would also prefer to add support for all major systems once and not part after part. The commands should be the following:
|
|
@ben-schwen thanks for outlining the commands. I’ll work on adding support for macOS and Linux using these and push an update soon. |
|
Hi @ben-schwen, while looking into the Linux clipboard side I noticed that newer distros (Ubuntu 22.04+, Fedora) default to Wayland, so I’m planning to add wl-paste alongside xclip/xsel to keep clipboard reading working smoothly there. For testing:
Let me know if you'd like me to do it differently or if you'd like me to add more commands. |
|
I've pushed an update adding clipboard support for macOS and Linux as discussed.
Let me know if there any adjustment are required. |
NEWS.md
Outdated
|
|
||
| 5. Non-equi joins combining an equality condition with two inequality conditions on the same column (e.g., `on = .(id == id, val >= lo, val <= hi)`) no longer error, [#7641](https://github.com/Rdatatable/data.table/issues/7641). The internal `chmatchdup` remapping of duplicate `rightcols` was overwriting the original column indices, causing downstream code to reference non-existent columns. Thanks @tarun-t for the report and fix, and @aitap for the diagnosis. | ||
|
|
||
| 6. `fread("clipboard")` now works on Linux (via `xclip`, `xsel`, or `wl-paste`), macOS (via `pbpaste`), and Windows, [#1292](https://github.com/Rdatatable/data.table/issues/1292). Thanks @mbacou for the report, @ben-schwen for the suggestion, and @AmanKashyap0807 for the fix. |
There was a problem hiding this comment.
pls elaborate more what this means, e.g. that fread supports now reading from the clipboard via fread("clipboard")
|
Let's not limit X11/Wayland clipboard support to just Linux. Yes, macOS
is special, but anything else where .Platform$OS.type == 'unix'
(FreeBSD, OpenBSD, NetBSD, Cygwin, OpenIndiana...) should work as well.
|
| } else { | ||
| warning("Clipboard reading is not supported on this platform.", call. = FALSE) | ||
| } | ||
| if (!length(clip) || !any(nzchar(trimws(clip)))) { |
There was a problem hiding this comment.
on non windows and non-unix this will error because of trimws(clip) but clip was never assigned
There was a problem hiding this comment.
I have now added return(data.table()) inside the else block for unsupported platforms.
|
Hi @ben-schwen and @aitap I have pushed changes addressing your comments
Please let me know if anything should be adjusted |
Closes #1292
Fix fread clipboard handling on Windows using
readClipboard(), add test (#2366), and update NEWS.Changed files :
clipboardinputs on Windows, reading it by using readClipboard(), handle errors and process clipboard content as a temporary file for parsing.To keep the scope of issue #1292, I only solved it for Windows. I'd be happy to submit another PR for macOS and Linux later.
Reproduction:
fread()does not recognize"clipboard"/"clipboard-128"as clipboard input on Windows. Due to the long period since the issue was reported, there might have been some changes in fread() over time, which is why the error message differs now, but the core problem is the same, clipboard input is not detected.Environment:
Please let me know if any changes are needed.